feat(package_info_plus)!: Migrate to js_interop for conditional export#2622
feat(package_info_plus)!: Migrate to js_interop for conditional export#2622amrgetment wants to merge 1 commit into
Conversation
amrgetment
changed the title
|
Why is the description saying "Fix for a device_info_plus ticket" but the code is Can you provide more information in the PR description regarding this change as well? |
|
@miquelbeltran it is a mistake, actually I use package info plus not device info plus |
miquelbeltran
left a comment
miquelbeltran
left a comment
There was a problem hiding this comment.
This is a bit confusing.
The package already uses web, so the ticket you created seems to be wrong. Can you elaborate on that?
| import 'package:http/http.dart'; | ||
| import 'package:package_info_plus_platform_interface/package_info_data.dart'; | ||
| import 'package:package_info_plus_platform_interface/package_info_platform_interface.dart'; | ||
| import 'package:web/helpers.dart' as web; |
There was a problem hiding this comment.
I think you need to leave helpers.dart, because we want to support web 0.3
There was a problem hiding this comment.
oh, I will revert it back if the PR open again
There was a problem hiding this comment.
reviewer made an alternative PR already
|
|
||
| export 'src/package_info_plus_linux.dart'; | ||
| export 'src/package_info_plus_windows.dart' | ||
| if (dart.library.html) 'src/package_info_plus_web.dart'; |
There was a problem hiding this comment.
Can you explain this change?
There was a problem hiding this comment.
according to this document you need to use js_interop for conditional import to support WASM,
am I wrong?
https://dart.dev/interop/js-interop/package-web
|
|
|
The migration happened a few months ago already #2316 |
|
@vbuberen according to this document you need to use js_interop for conditional import
so are you sure it is compatible with WASM with dart.library.html? |
|
I am sure, because if you would check the link right above your comment you would see that I created another PR with the same change, but took a few minutes to provide a proper description with links instead of what was provided in this PR. 
|
|
@vbuberen I replied to this
we have a different time zones so I replied late as I made this PR night and I checked it noontime |
|
I have just explained to you why you shouldn't tag people and you still continue 🤦🏻
It doesn't matter. In case PR had a proper description with links (like the one that I created instead) and didn't reference an issue |
|
Next time please provide a clear and full description as people might not be in the context of what you have in your head. With that being said locking this discussion as there is nothing to discuss more. |
3 participants
Description
I removed html for conditional import and used js_interop to support WASM
Related Issues
Checklist
CHANGELOG.mdnor the plugin version inpubspec.yamlfiles.flutter analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
!in the title as explained in Conventional Commits).